Skip to content

[libcxx] Update picolibc version and remove xfails due to picolibc's support for char16_t and char32_t #114422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

vhscampos
Copy link
Member

@vhscampos vhscampos commented Oct 31, 2024

picolibc recently
(link) added support for char16_t and char32_t.

These xfails aren't needed anymore.

The picolibc version has been updates to v1.8.9, a version that includes the fix linked above.

@vhscampos vhscampos requested a review from a team as a code owner October 31, 2024 16:04
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 31, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-libcxx

Author: Victor Campos (vhscampos)

Changes

picolibc recently
(link) added support for char16_t and char32_t.

These xfails aren't needed anymore.


Full diff: https://github.com/llvm/llvm-project/pull/114422.diff

2 Files Affected:

  • (modified) libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp (-3)
  • (modified) libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp (-3)
diff --git a/libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp b/libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp
index a1560c8ee5853c..2b64554666817d 100644
--- a/libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp
+++ b/libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp
@@ -11,9 +11,6 @@
 // Apple platforms don't provide <uchar.h> yet, so these tests fail.
 // XFAIL: target={{.+}}-apple-{{.+}}
 
-// mbrtoc16 not defined.
-// XFAIL: LIBCXX-PICOLIBC-FIXME
-
 // <uchar.h>
 
 #include <uchar.h>
diff --git a/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp b/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp
index 2076384deb2b23..db00cbde333658 100644
--- a/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp
+++ b/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp
@@ -11,9 +11,6 @@
 // Apple platforms don't provide <uchar.h> yet, so these tests fail.
 // XFAIL: target={{.+}}-apple-{{.+}}
 
-// mbrtoc16 not defined.
-// XFAIL: LIBCXX-PICOLIBC-FIXME
-
 // <cuchar>
 
 #include <cuchar>

@vhscampos
Copy link
Member Author

The test failures seem to be present in other Pull Requests on the libcxx component, so I believe they are spurious or unrelated to this specific patch.

@vhscampos
Copy link
Member Author

It seems that this patch can only land after the CI pipelines update their picolibc to a version that contains picolibc/picolibc@f6f6875 as this was when support for char16_t and char32_t was introduced.

@ldionne
Copy link
Member

ldionne commented Nov 1, 2024

@mplatings Can you please have a look?

@ldionne
Copy link
Member

ldionne commented Nov 1, 2024

@vhscampos It might be sufficient to do this:

diff --git a/libcxx/utils/ci/build-picolibc.sh b/libcxx/utils/ci/build-picolibc.sh
index 521c1bef9fc7..6913d1d15a1a 100755
--- a/libcxx/utils/ci/build-picolibc.sh
+++ b/libcxx/utils/ci/build-picolibc.sh
@@ -70,7 +70,7 @@ picolibc_build_dir="${build_dir}/picolibc-build"
 mkdir -p "${picolibc_source_dir}"
 mkdir -p "${picolibc_build_dir}"
 # Download a known good version of picolibc.
-picolibc_commit="48fbc2009c6473293d03d5ec6f190565c6223a5c"
+picolibc_commit="f6f68758fe2aa62854f49b90c3daeeb08242b512"
 curl -L "https://github.com/picolibc/picolibc/archive/${picolibc_commit}.zip" --output "${picolibc_source_dir}/picolibc.zip"
 unzip -q "${picolibc_source_dir}/picolibc.zip" -d "${picolibc_source_dir}"
 mv "${picolibc_source_dir}/picolibc-${picolibc_commit}"/* "${picolibc_source_dir}"

@dcandler
Copy link
Collaborator

dcandler commented Nov 1, 2024

Updating the picolibc commit will potentially affect other tests due in bringing in more changes. My suggestion was to wait for the next release of picolibc so that a stable version can be used, then update all the tests for that.

@ldionne
Copy link
Member

ldionne commented Nov 1, 2024

I'd be fine with that. When's the next release of picolibc planned?

@ldionne
Copy link
Member

ldionne commented Jan 13, 2025

Gentle ping on this patch.

@vhscampos
Copy link
Member Author

@ldionne I am still waiting on the next release of picolibc. I should receive a notification from GitHub once it's done, then I can get this PR merged.

…r32_t

picolibc recently
([link](picolibc/picolibc@f6f6875))
added support for char16_t and char32_t.

These xfails aren't needed anymore.
@vhscampos
Copy link
Member Author

@ldionne
We got a new picolibc release: 1.8.9.

PR to bump the version: #126074

@philnik777
Copy link
Contributor

This and the bump should probably land together, since the CI will fail otherwise.

@vhscampos vhscampos force-pushed the libcxx-picolibc-char-remove-xfails branch from 2c6b08b to ecb4325 Compare February 6, 2025 14:20
@vhscampos vhscampos changed the title [libcxx] Remove xfails due to picolibc's support for char16_t and char32_t [libcxx] Update picolibc version and remove xfails due to picolibc's support for char16_t and char32_t Feb 6, 2025
@vhscampos
Copy link
Member Author

Updating to v1.8.9 causes several failures related to locale. So this can't be merged until the failures are investigated and fixed.

@ldionne
Copy link
Member

ldionne commented May 12, 2025

Temporarily closing the PR to clear up the review queue. Please reopen when you pick up this task again.

@ldionne ldionne closed this May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants